Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use CaptureContext class to simplify code in hooks #13

Merged
merged 5 commits into from
Nov 23, 2015

Conversation

ioannis-e
Copy link

Document special case in vld_unload test
Enable capturing vector_new... in msvcrtPatch
Skip stack trace for frames internal to vld
Use CaptureContext class to simplify code in hooks

@KindDragon
Copy link
Owner

😢 https://ci.appveyor.com/project/KindDragon/vld/build/140/job/4m8viq3lfecrlcpf/tests
I think that this bug not always reproduced. I'll try to rebuild it https://ci.appveyor.com/project/KindDragon/vld/build/141 , I don't know how fix it

@KindDragon
Copy link
Owner

https://ci.appveyor.com/project/KindDragon/vld/build/146/job/vc1g5ofkc7ee05ut a lot of messages "VLD: New allocation at already allocated address: 0x00E09700 with size: 164 and new size: 164"

@ioannis-e
Copy link
Author

I think the root of the failure with the following builds is that the debug symbols for mfc120ud.dll are not loaded for some reason, and therefore the frame pointers are not resolved properly.
https://ci.appveyor.com/project/KindDragon/vld/build/146
https://ci.appveyor.com/project/KindDragon/vld/build/147

If you take a look at the following call stack, taken from the above builds, before the call to MSVCR120D.dll!initterm_e() there should be a call to _CRT_INIT which we would normally catch and ignore.

Never the less skipping startup CRT allocations depends on all the debug symbols being loaded, so we should investigate why these symbols were not loaded in the first place.

[00:00:49] ---------- Block 6635 at 0x00DF2038: 3722304989 bytes ----------
[00:00:49]   Leak Hash: 0x64A52A3C, Count: 1, Total 3722304989 bytes
[00:00:49]   Call Stack (TID 628):
[00:00:49]     MSVCR120D.dll!malloc_dbg()
[00:00:49]     mfc120ud.dll!0x712CE2AB()
[00:00:49]     MSVCR120D.dll!initterm_e() + 0x28 bytes
[00:00:49]     mfc120ud.dll!0x712CE3C6()
[00:00:49]     mfc120ud.dll!0x712CE6DC()
[00:00:49]     mfc120ud.dll!0x712CE61F()
[00:00:49]     c:\projects\vld\src\vld.cpp (99): vld_x86.dll!LdrpCallInitRoutine() + 0xC bytes
[00:00:49]     ntdll.dll!RtlInitializeCriticalSection() + 0x10E bytes
[00:00:49]     ntdll.dll!RtlInitializeCriticalSection() + 0x88 bytes
[00:00:49]     ntdll.dll!RtlIsCriticalSectionLockedByThread() + 0x2A5 bytes
[00:00:49]     ntdll.dll!RtlIsCriticalSectionLockedByThread() + 0x1ED bytes
[00:00:49]     ntdll.dll!RtlGetVersion() + 0x7C0 bytes
[00:00:49]     ntdll.dll!RtlIsCriticalSectionLockedByThread() + 0x155 bytes
[00:00:49]     ntdll.dll!LdrResFindResourceDirectory() + 0x210 bytes
[00:00:49]     ntdll.dll!LdrLoadDll() + 0x6F bytes
[00:00:49]     KERNELBASE.dll!LoadLibraryExW() + 0xC6 bytes
[00:00:49]     KERNEL32.DLL!LoadLibraryW() + 0x12 bytes
[00:00:49]     c:\projects\vld\src\tests\dynamic_app\loadtests.cpp (86): dynamic_app.exe!RunMFCLoaderTests() + 0xD bytes
[00:00:49]     c:\projects\vld\src\tests\dynamic_app\threadtest.cpp (27): dynamic_app.exe!Call_LoaderLocks() + 0xC bytes
[00:00:49]     c:\projects\vld\src\tests\dynamic_app\threadtest.cpp (44): dynamic_app.exe!Call_Three() + 0x9 bytes
[00:00:49]     c:\projects\vld\src\tests\dynamic_app\threadtest.cpp (49): dynamic_app.exe!Call_Two() + 0x9 bytes
[00:00:49]     c:\projects\vld\src\tests\dynamic_app\threadtest.cpp (54): dynamic_app.exe!Call_One() + 0x9 bytes
[00:00:49]     c:\projects\vld\src\tests\dynamic_app\threadtest.cpp (60): dynamic_app.exe!Dynamic_Thread_Procedure() + 0x9 bytes
[00:00:49]     MSVCR120D.dll!beginthreadex() + 0x1A1 bytes
[00:00:49]     MSVCR120D.dll!endthreadex() + 0x181 bytes
[00:00:49]     KERNEL32.DLL!BaseThreadInitThunk() + 0x24 bytes
[00:00:49]     ntdll.dll!RtlInitializeExceptionChain() + 0x8F bytes
[00:00:49]     ntdll.dll!RtlInitializeExceptionChain() + 0x5A bytes
[00:00:49]   Data:
[00:00:49]     DD DD DD DD    DD DD DD DD    DD DD DD DD    DD DD DD DD     ........ ........
[00:00:49]     DD DD DD DD    DD DD DD DD    DD DD DD DD    DD DD DD DD     ........ ........
[00:00:49]     DD DD DD DD    DD DD DD DD    DD DD DD DD    DD DD DD DD     ........ ........
[00:00:49]     DD DD DD DD    DD DD DD DD    DD DD DD DD    DD DD DD DD     ........ ........
[00:00:49]     DD DD DD DD    DD DD DD DD    DD DD DD DD    DD DD 99 7E     ........ .......~
[00:00:49]     49 1B 5A 59    73 01 00 8C    A8 B0 E1 00    08 19 DD 00     I.ZYs... ........
[00:00:49]     88 DA D7 70    9C 00 00 00    40 00 00 00    01 00 00 00     ...p.... @.......
[00:00:49]     BD 22 00 00    FD FD FD FD    A0 D7 35 71    17 00 00 00     ."...... ..5q....
[00:00:49]     17 00 00 00    01 00 00 00    34 00 20 00    70 00 61 00     ........ 4...p.a.
[00:00:49]     72 00 61 00    6D 00 65 00    74 00 65 00    72 00 20 00     r.a.m.e. t.e.r...
[00:00:49]     6E 00 65 00    77 00 20 00    66 00 6F 00    72 00 20 00     n.e.w... f.o.r...
[00:00:49]     6D 00 66 00    63 00 00 00    FD FD FD FD    DD DD DD DD     m.f.c... ........
[00:00:49]     47 1B 68 59    DD 02 00 8C    F8 B1 E1 00    C8 15 DD 00     G.hY.... ........
[00:00:49]     88 DA D7 70    9C 00 00 00    40 00 00 00    01 00 00 00     ...p.... @.......
[00:00:49]     71 22 00 00    FD FD FD FD    A0 D7 35 71    17 00 00 00     q"...... ..5q....
[00:00:49]     17 00 00 00    01 00 00 00    34 00 20 00    70 00 61 00     ........ 4...p.a.

@KindDragon
Copy link
Owner

Hmm, appveyor always reproduce this bug with VldStackWalkMethod=fast, Toolset=v120_xp

@ioannis-e
Copy link
Author

Maybe instead of file paths we should test for function names that are called within startup crt and make allocations ?
I'll make some tests to see if we get better results.

Did you manage to understand why these messages arise?
"VLD: New allocation at already allocated address: 0x00E09700 with size: 164 and new size: 164"

@KindDragon
Copy link
Owner

I will try reproduce it locally

@KindDragon
Copy link
Owner

VLD: New allocation at already allocated address: 0x013CA708 with size: 164 and new size: 164
  TID: 12604
  Call Stack:
    f:\dd\vctools\crt\crtw32\misc\dbgheap.c (159): MSVCR120D.dll!_malloc_dbg()
    f:\dd\vctools\crt\crtw32\dllstuff\crtdll.c (163): mfc120ud.dll!pre_c_init() + 0x17 bytes
    f:\dd\vctools\crt\crtw32\startup\crt0dat.c (1005): MSVCR120D.dll!_initterm_e() + 0x7 bytes
    f:\dd\vctools\crt\crtw32\dllstuff\crtdll.c (287): mfc120ud.dll!_CRT_INIT() + 0xF bytes
    f:\dd\vctools\crt\crtw32\dllstuff\crtdll.c (502): mfc120ud.dll!__DllMainCRTStartup() + 0x11 bytes
    f:\dd\vctools\crt\crtw32\dllstuff\crtdll.c (472): mfc120ud.dll!_DllMainCRTStartup() + 0x11 bytes
    d:\work\vld\src\vld.cpp (99): vld_x86.dll!LdrpCallInitRoutine() + 0xC bytes
    ntdll.dll!LdrxCallInitRoutine() + 0x16 bytes
    ntdll.dll!LdrpCallInitRoutine() + 0x43 bytes
    ntdll.dll!LdrpInitializeNode() + 0x101 bytes
    ntdll.dll!LdrpInitializeGraphRecurse() + 0x71 bytes
    ntdll.dll!LdrpInitializeGraphRecurse() + 0x88 bytes
    ntdll.dll!LdrpPrepareModuleForExecution() + 0x8B bytes
    ntdll.dll!LdrpLoadDllInternal() + 0x121 bytes
    ntdll.dll!LdrpLoadDll() + 0x92 bytes
    ntdll.dll!LdrLoadDll() + 0xD9 bytes
    KERNELBASE.dll!LoadLibraryExW() + 0x138 bytes
    KERNELBASE.dll!LoadLibraryW() + 0x11 bytes
    d:\work\vld\src\tests\dynamic_app\loadtests.cpp (86): dynamic_app.exe!RunMFCLoaderTests() + 0xD bytes
    d:\work\vld\src\tests\dynamic_app\threadtest.cpp (27): dynamic_app.exe!Call_LoaderLocks() + 0xC bytes
    d:\work\vld\src\tests\dynamic_app\threadtest.cpp (44): dynamic_app.exe!Call_Three() + 0x9 bytes
    d:\work\vld\src\tests\dynamic_app\threadtest.cpp (49): dynamic_app.exe!Call_Two() + 0x9 bytes
    d:\work\vld\src\tests\dynamic_app\threadtest.cpp (54): dynamic_app.exe!Call_One() + 0x9 bytes
    d:\work\vld\src\tests\dynamic_app\threadtest.cpp (60): dynamic_app.exe!Dynamic_Thread_Procedure() + 0x9 bytes
    f:\dd\vctools\crt\crtw32\startup\threadex.c (376): MSVCR120D.dll!_callthreadstartex() + 0xF bytes
    f:\dd\vctools\crt\crtw32\startup\threadex.c (359): MSVCR120D.dll!_threadstartex()
    KERNEL32.DLL!BaseThreadInitThunk() + 0x24 bytes
    ntdll.dll!__RtlUserThreadStart() + 0x2F bytes
    ntdll.dll!_RtlUserThreadStart() + 0x1B bytes

@KindDragon
Copy link
Owner

I think it's bug in v120 CRT

@ioannis-e
Copy link
Author

I'm trying some chages in 6e23df9
Waiting for appyeor results

@KindDragon
Copy link
Owner

One of two 😄

@ioannis-e
Copy link
Author

👍

KindDragon added a commit that referenced this pull request Nov 23, 2015
Use CaptureContext class to simplify code in hooks
@KindDragon KindDragon merged commit af90f6b into KindDragon:master Nov 23, 2015
@ioannis-e
Copy link
Author

Will you release the RC now, or are there any more pending matters ?

@KindDragon
Copy link
Owner

We have new false positive leaks in VS2015 😢 https://gist.github.com/akaStiX/fde381ec7e29a7264b9b

@ioannis-e
Copy link
Author

Can you include a test case for https://gist.github.com/akaStiX/fde381ec7e29a7264b9b maybe in vld_main ?

I think adding || beginWith(functionName, len, L"std::_Crt_new_delete::operator new")
in CallStack::isCrtStartupFunction should do the trick, but without a test case, i wouldn't know how it affects all configurations

@KindDragon
Copy link
Owner

Can you include a test case for https://gist.github.com/akaStiX/fde381ec7e29a7264b9b maybe in vld_main ?

I can't reproduce it ATM in vld tests 😢

@ioannis-e
Copy link
Author

Try in vld_main the following code

#include <iostream>
int Test()
{
    std::cout << "cout";
    std::cerr << "cerr";
    ...
}

@ioannis-e
Copy link
Author

Does VS2015 std library dynamically initialize any other objects appart from the ones below ?

std::`dynamic initializer for 'cout''
std::`dynamic initializer for 'fout''
std::`dynamic initializer for 'cerr''
std::`dynamic initializer for 'ferr''

We could simply use the following to exclude all std dynamic initializations
|| beginWith(functionName, len, L"std::dynamic initializer for '")`

@ioannis-e
Copy link
Author

Tested the above and it seems to work perfectly :)
Do you want to make the change ?

@KindDragon
Copy link
Owner

Do you want to make the change ?

I'll do later

@KindDragon
Copy link
Owner

Try in vld_main the following code

Maybe I'm doing something wrong, but I can't reproduce it.

@ioannis-e
Copy link
Author

VS2015, v140, Release_StaticCrt

@ioannis-e ioannis-e deleted the PR branch November 25, 2015 13:27
@KindDragon
Copy link
Owner

This commit 1ad587e should fix issue

@ioannis-e
Copy link
Author

I was working on cleaning up 3 commits
https://github.com/ioannis-e/vld/commits/develop
Can you take a look ?

@KindDragon
Copy link
Owner

Changes from 120d622 no longer needed

@ioannis-e
Copy link
Author

I'd suggest changes in bool CallStack::isCrtStartupAlloc() and int CallStack::resolve(BOOL showInternalFrames) they eliminate some control paths
In any case cherry pick what you like

@ioannis-e
Copy link
Author

Does 120d622 resolve this ? Note that this is with VS2013 with the code posted above.

---------- Block 67 at 0x00701770: 24 bytes ----------
  Leak Hash: 0x8320BEEF, Count: 1, Total 24 bytes
  Call Stack (TID 1252):
    ntdll.dll!RtlAllocateHeap()
    f:\dd\vctools\crt\crtw32\heap\malloc.c (92): vld_main.exe!malloc() + 0x3B bytes
    f:\dd\vctools\crt\crtw32\heap\new.cpp (59): vld_main.exe!operator new() + 0x8 bytes
    f:\dd\vctools\crt\crtw32\stdhpp\xlocale (2473): vld_main.exe!std::ctype<char>::_Getcat() + 0x7 bytes
    f:\dd\vctools\crt\crtw32\stdhpp\xlocale (578): vld_main.exe!std::use_facet<std::ctype<char> >() + 0xC bytes
    f:\dd\vctools\crt\crtw32\stdhpp\ios (127): vld_main.exe!std::basic_ios<char,std::char_traits<char> >::widen() + 0x13 bytes
    f:\dd\vctools\crt\crtw32\stdhpp\ios (172): vld_main.exe!std::basic_ios<char,std::char_traits<char> >::init()
    f:\dd\vctools\crt\crtw32\stdhpp\ostream (56): vld_main.exe!std::basic_ostream<char,std::char_traits<char> >::basic_ostream<char,std::char_traits<char> >()
    f:\dd\vctools\crt\crtw32\stdcpp\cout.cpp (16): vld_main.exe!std::`dynamic initializer for 'cout''() + 0x13 bytes
    f:\dd\vctools\crt\crtw32\startup\crt0dat.c (321): vld_main.exe!_cinit()
    f:\dd\vctools\crt\crtw32\startup\crt0.c (237): vld_main.exe!__tmainCRTStartup() + 0x7 bytes
    kernel32.dll!BaseThreadInitThunk() + 0xE bytes
    ntdll.dll!__RtlUserThreadStart() + 0x70 bytes
    ntdll.dll!_RtlUserThreadStart() + 0x1B bytes
  Data:
    F0 F2 F6 00    01 00 00 00    00 00 00 00    80 19 70 00     ........ ......p.
    01 00 00 00    00 00 00 00                                   ........ ........

@KindDragon
Copy link
Owner

Does 120d622 resolve this ? Note that this is with VS2013 with the code posted above.

I'll try your code again

@ioannis-e
Copy link
Author

no need to include both cout and cerr. Just try cout, as cerr may interfere with appveyor

@KindDragon
Copy link
Owner

It's only in Release with StaticCrt https://ci.appveyor.com/project/KindDragon/vld/build/161

@ioannis-e
Copy link
Author

Yes the fix with ucrt is only for debug mode!
Maybe we should rename the function to properly reflect what is being handled ie isReleaseStaticCrtStartupFunction hehe

As i thought appveyor reports cerr to its console in red

vld_main.exe : cerr
At line:43 char:9
+         & $_.FullName "--gtest_output=`"xml:$testfile`""
+         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : NotSpecified: (cerr:String) [], RemoteException
    + FullyQualifiedErrorId : NativeCommandError

@KindDragon
Copy link
Owner

@ioannis-e gitter chat to simplify discussion on issues https://gitter.im/KindDragon/vld

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants